-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update clang to upstream chromium m114's clang-17 #2680
Conversation
b77b65b
to
9e5dbac
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2680 +/- ##
==========================================
+ Coverage 58.82% 58.86% +0.03%
==========================================
Files 1906 1906
Lines 93605 93605
==========================================
+ Hits 55060 55096 +36
+ Misses 38545 38509 -36 ☔ View full report in Codecov by Sentry. |
9e5dbac
to
ace015d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets clean up the variable names so we don't have churn later.
I'd also consider reducing some duplication maybe, e.g. move the install code into a shared .sh
file between the two containers, but that's up to you. It's more of a separate refactoring problem.
ace015d
to
9135ea1
Compare
done, will carry out the refactor in a follow up |
9135ea1
to
2b473a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay - please be very careful while landing this, as it impacts a lot of containers.
agreed, my plan is to test this out on internal builds before pushing with @briantting 's help. |
c7186e4
to
2646ab9
Compare
Change-Id: I5faeaae796d0b6d50e590d1267c3512a265a9f9a
Change-Id: I1038f7d435660ad39036b177f375ed3d9529562b
2646ab9
to
c3c5032
Compare
This reverts commit 92151b6.
b/323184809